Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

refactor: Chunk.modules => Chunk.mapModules (webpack v3.0.0) #543

Closed

Conversation

MirrorBytes
Copy link
Contributor

@MirrorBytes MirrorBytes commented Jun 13, 2017

Refactors to use webpack 3.x Chunk.mapModules

Closes #529

Squashed commit message body for merge


- refactor: DeprecationWarning: Chunk.modules

BREAKING CHANGE: Updates to `Chunk.mapModules`. This release is not backwards compatible with `Webpack 2.x` due to breaking changes in webpack/webpack#4764

@@ -269,7 +269,7 @@ ExtractTextPlugin.prototype.apply = function(compiler) {
async.forEach(chunks, function(chunk, callback) {
var extractedChunk = extractedChunks[chunks.indexOf(chunk)];
var shouldExtract = !!(options.allChunks || isInitialOrHasNoParents(chunk));
async.forEach(chunk.modules.slice(), function(module, callback) {
async.forEach(chunk.mapModules(function(c) { return c; }), function(module, callback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having just refactored this entire plugin, there are 4/5 instances of chunk.modules.something

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about function (m) { return m; } for precise initials?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

c === chunk the naming convention is as it should be

Copy link

@JLHwung JLHwung Jun 24, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mapModules map functions on each module so the iteratee will be called with each module. See https://github.com/webpack/webpack/pull/4764/files#diff-8941681d920ac4feda44f522b986c2d0R115

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Jun 13, 2017

This will land in the v2.x line fixing the Deprecation Warning , before webpack-defaults (v3.0.0) lands? :) I'm a bit confused 😛

@joshwiens
Copy link
Member

@michael-ciniawsky - This change is not backwards compatible with 2.x hence why it's targeting the features/webpack3.

This can merge into that target branch when ready & I can rebase in the changes to the defaults PR.

@MirrorBytes
Copy link
Contributor Author

@d3viant0ne I'll change all of the instances accordingly. Sound good?

@michael-ciniawsky michael-ciniawsky changed the title fix(index): DeprecationWarning: Chunk.modules in Webpack 3.x fix(index): Chunk.modules => Chunk.mapModules (webpack v3.0.0) Jun 20, 2017
@michael-ciniawsky
Copy link
Member

@MirrorBytes What is missing here, did you find the remaining places, where this needs to be updated as @d3viant0ne suggested?

@michael-ciniawsky michael-ciniawsky added this to the 3.0.0 milestone Jun 20, 2017
@michael-ciniawsky michael-ciniawsky changed the title fix(index): Chunk.modules => Chunk.mapModules (webpack v3.0.0) breaking(index): Chunk.modules => Chunk.mapModules (webpack v3.0.0) Jun 20, 2017
@alexander-akait
Copy link
Member

@michael-ciniawsky maybe we can check webpack version and use difference behavior without breaking changes?

@michael-ciniawsky
Copy link
Member

@evilebottnawi How would this look like ? There is this.version (at least in the Loader API), so maybe

function mapModules (chunk) {
   if (this.version >= 3) return chunk.mapModule((c) => c)
   return chunk.modules.slice()
}

or check the chunk for extisting methods would work 😛

@alexander-akait
Copy link
Member

@michael-ciniawsky seems good, but need wait what says @d3viant0ne

@michael-ciniawsky
Copy link
Member

Fixes #494

@joshwiens joshwiens changed the title breaking(index): Chunk.modules => Chunk.mapModules (webpack v3.0.0) refactor: Chunk.modules => Chunk.mapModules (webpack v3.0.0) Jun 21, 2017
@joshwiens
Copy link
Member

@MirrorBytes - This will need to be rebased now that the defaults PR has landed.

Don't know how good you are with git, if you aren't comfortable doing it just give me write permission ( hit edit in this pr & you should see a check box ) and I will rebase it for you.

@themre
Copy link

themre commented Jun 22, 2017

I hear appveyor has some issues lately. perhaps someone else can run it on windows to release it.

@bobvanderlinden
Copy link

I rebased this PR on feature/webpack3 and changed the remaining references to Chunk.modules to Chunk.mapModules. The resulting version can be found here: https://github.com/bobvanderlinden/extract-text-webpack-plugin/commits/pr-543

With the additional changes, the test does not output any warnings like it did before:

(node:3880) DeprecationWarning: Chunk.modules is deprecated. Use Chunk.getNumberOfModules/mapModules/forEachModule/containsModule instead.

I ran the tests on Windows 10, node v6.4.0, webpack 3.0.0.
1 test failed:

Expected value to equal:
  "body {
        correct: a;
}
body {
        correct: b;
}

/*# sourceMappingURL=file.css.map*/"
Received:
  "body {
        correct: a;
}
body {
        correct: b;
}

/*# sourceMappingURL=file.css.map*/"

Difference:

- Expected
+ Received

@@ -2,7 +2,7 @@
        correct: a;
 }
 body {
        correct: b;
 }
-
+
 /*# sourceMappingURL=file.css.map*/

Note though that this seems unrelated to this PR as the test also fails on feature/webpack3. I'm guessing it's an \r\n vs \n problem?

@joshwiens
Copy link
Member

I'll try to take a look at the windows issue tonight.

@c0b41
Copy link

c0b41 commented Jun 28, 2017

any status?

@cklmercer
Copy link

Not sure if this is related but I recently started encountering the following issue on Windows 10.

TypeError: chunk.mapModules is not a function
    at C:\Users\me\Code\xxx\xxx.com\node_modules\webpack\lib\HotModuleReplacementPlugin.js:59:46
    at Array.forEach (native)
    at Compilation.<anonymous> (C:\Users\me\Code\xxx\xxx.com\node_modules\webpack\lib\HotModuleReplacementPlugin.js:58:16)
    at Compilation.applyPlugins2 (C:\Users\me\Code\xxx\xxx.com\node_modules\tapable\lib\Tapable.js:82:14)
    at sealPart2 (C:\Users\me\Code\xxx\xxx.com\node_modules\webpack-dev-server\node_modules\webpack\lib\Compilation.js:629:10)
    at next (C:\Users\me\Code\xxx\xxx.com\node_modules\tapable\lib\Tapable.js:138:11)
    at Compilation.compilation.plugin (C:\Users\me\Code\xxx\xxx.com\node_modules\webpack-dev-server\node_modules\webpack\lib\ProgressPlugin.js:109:6)
    at Compilation.applyPluginsAsyncSeries (C:\Users\me\Code\xxx\xxx.com\node_modules\tapable\lib\Tapable.js:142:13)
    at Compilation.seal (C:\Users\me\Code\xxx\xxx.com\node_modules\webpack-dev-server\node_modules\webpack\lib\Compilation.js:579:8)
    at C:\Users\me\Code\xxx\xxx.com\node_modules\webpack-dev-server\node_modules\webpack\lib\Compiler.js:493:16

@joshwiens
Copy link
Member

This has been taken care of in another PR given the time pressure to get etwp stable in Webpack 3.x

@joshwiens joshwiens closed this Jul 7, 2017
joshwiens added a commit that referenced this pull request Jul 10, 2017
- refactor: Pass a unique compiler name to get child compilation [483](#483)
- refactor: Apply webpack-defaults [542](#542)

BREAKING CHANGE: Enforces `engines` of `"node": ">=4.3.0 < 5.0.0 || >= 5.10`

- refactor: DeprecationWarning: Chunk.modules [543](#543)

BREAKING CHANGE: Updates to `Chunk.mapModules`. This release is not backwards compatible with `Webpack 2.x` due to breaking changes in webpack/webpack#4764

- fix: css generation order issue see: webpack/webpack#5225

BREAKING CHANGE: Enforces `peerDependencies` of `"webpack": "^3.1.0"`.
@michael-ciniawsky michael-ciniawsky modified the milestone: 4.0.0 Aug 31, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants